fix(clerk-sdk-node): Inherit verifyToken options from clerkClient by panteliselef · Pull Request #3296 · clerk/javascript

panteliselef · 2024-05-01T11:12:27Z

Description

This PR allows clerkClient.verifyToken from "@clerk/clerk-sdk-node" to inherit the VerifyTokenOptions set when clerkClient is instantiated.

Added an additional error when both jwtKey and secretKey are missing.

fixes #3283

Checklist

npm test runs as expected.
npm run build runs as expected.
(If applicable) JSDoc comments have been added or updated for any package exports
(If applicable) Documentation has been updated

Type of change

changeset-bot · 2024-05-01T11:12:31Z

🦋 Changeset detected

Latest commit: 5782b00

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@clerk/clerk-sdk-node	Patch
@clerk/backend	Patch
@clerk/express	Patch
@clerk/fastify	Patch
@clerk/nextjs	Patch
@clerk/remix	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

panteliselef · 2024-05-01T11:12:41Z

!snapshot

clerk-cookie · 2024-05-01T11:17:00Z

Hey @panteliselef - the snapshot version command generated the following package versions:

Package	Version
@clerk/backend	1.1.2-snapshot.v9a7208b
@clerk/chrome-extension	1.0.4-snapshot.v9a7208b
@clerk/clerk-js	5.2.1-snapshot.v9a7208b
@clerk/clerk-expo	1.0.4-snapshot.v9a7208b
@clerk/express	0.0.5-snapshot.v9a7208b
@clerk/fastify	1.0.4-snapshot.v9a7208b
gatsby-plugin-clerk	5.0.0-beta.45
@clerk/nextjs	5.0.5-snapshot.v9a7208b
@clerk/remix	4.0.4-snapshot.v9a7208b
@clerk/clerk-sdk-node	5.0.4-snapshot.v9a7208b

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/backend

npm i @clerk/backend@1.1.2-snapshot.v9a7208b --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@1.0.4-snapshot.v9a7208b --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.2.1-snapshot.v9a7208b --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@1.0.4-snapshot.v9a7208b --save-exact

@clerk/express

npm i @clerk/express@0.0.5-snapshot.v9a7208b --save-exact

@clerk/fastify

npm i @clerk/fastify@1.0.4-snapshot.v9a7208b --save-exact

gatsby-plugin-clerk

npm i gatsby-plugin-clerk@5.0.0-beta.45 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@5.0.5-snapshot.v9a7208b --save-exact

@clerk/remix

npm i @clerk/remix@4.0.4-snapshot.v9a7208b --save-exact

@clerk/clerk-sdk-node

npm i @clerk/clerk-sdk-node@5.0.4-snapshot.v9a7208b --save-exact

panteliselef · 2024-05-01T12:40:07Z

+      if (!options.secretKey || !options.jwtKey) {
+        return {
+          errors: [
+            new TokenVerificationError({
+              action: TokenVerificationErrorAction.SetClerkSecretKey,
+              message: 'Both JWT Key and Secret Key are missing. Operation could not be completed.',
+              reason: TokenVerificationErrorReason.InvalidSecretKey,
+            }),
+          ],
+        };
+      }
+


Do we think that this is a breaking change ? Tbh i think we can skip its introduction

We could also just throw a warning instead so we don't introduce a breaking change

panteliselef · 2024-05-01T12:40:46Z

+```ts
+import { clerkClient } from "@clerk/clerk-sdk-node";
+
+clerkClient.verifyToken(token, {})


Only reason, i've not update the signature is that this package will be deprecated.

dimkl

☁️ I understand that the customer use case is valid and that we probably need to make a change to clerkClient.verifyToken(token, options) to support execution without passing options but i am not 100% sure that we should make it this way.
I would expect to either keep it as is and inform the customers using it explicitly that it's just a re-export from the @clerk/backend and all the arguments required should be passed. or support the following:

import { clerkClient } from "@clerk/clerk-sdk-node";

// case #1: use options passed in clerkClient
clerkClient.verifyToken(token)

// case #2: options should be non-conflicting with the ones passed in clerkClient
// as it does not make sense to have a different jwtKey set in the clerkClient and a different one used in verifyToken
clerkClient.verifyToken(token, options)

If they need to pass explicitly the options i would advise to either create a different clerkClient or use the verifytoken from the @clerk/backend.
I think we should go for ONLY the 1st case. We should check if the @clerk/express supports the same API and make the appropriate changes there.
cc: @nikosdouvlis

panteliselef · 2024-05-09T06:18:27Z

@dimkl
Regarding case 1, Completely dropping the 2nd params, wouldn't that be a breaking change ?

If we are going the extra mile here, let's still support the 2nd param, but update the signature so that 2nd param is optional. It is the least disruptive solution.

Your suggestions make sense, but I would skip them for this package and only apply them to the new express package

…-during-verification

shadoworion · 2024-05-10T11:41:49Z

☁️ I understand that the customer use case is valid and that we probably need to make a change to clerkClient.verifyToken(token, options) to support execution without passing options but i am not 100% sure that we should make it this way. I would expect to either keep it as is and inform the customers using it explicitly that it's just a re-export from the @clerk/backend and all the arguments required should be passed. or support the following:
import { clerkClient } from "@clerk/clerk-sdk-node";

// case #1: use options passed in clerkClient
clerkClient.verifyToken(token)

// case #2: options should be non-conflicting with the ones passed in clerkClient
// as it does not make sense to have a different jwtKey set in the clerkClient and a different one used in verifyToken
clerkClient.verifyToken(token, options)
If they need to pass explicitly the options i would advise to either create a different clerkClient or use the verifytoken from the @clerk/backend. I think we should go for ONLY the 1st case. We should check if the @clerk/express supports the same API and make the appropriate changes there. cc: @nikosdouvlis

Hi, I use this function to create auth middleware for the "graphql-yoga" server. All options are default and secret from env (which doesn't work now), so "case #1" fits better.

If I use different options that conflict with the current "clerkClient," I'll use "createClerkClient" to create a new one (that also doesn't work now)

dimkl · 2024-05-10T11:55:53Z

+  return (...args: Parameters<VerifyTokenWithOptionalSecondArgument>) =>
+    _verifyToken(args[0], {
+      ...params,
+      ...args[1],


🔧 Could you add 2 tests (with options, without options) to verify that everything works as expected?

panteliselef · 2024-05-10T17:23:16Z

-const mockNext = jest.fn();
-
-afterEach(() => {
-  mockNext.mockReset();
-});
-


@dimkl fyi, Just cleaning this up

…-during-verification

fix(clerk-sdk-node): Inherit verifyToken options from clerkClient

9a7208b

panteliselef requested review from dimkl and nikosdouvlis May 1, 2024 11:12

panteliselef self-assigned this May 1, 2024

clerk-cookie added backend sdk-node labels May 1, 2024

panteliselef requested a review from brkalow May 1, 2024 12:36

panteliselef commented May 1, 2024

View reviewed changes

dimkl reviewed May 8, 2024

View reviewed changes

dimkl reviewed May 10, 2024

View reviewed changes

Comment thread packages/backend/src/tokens/verify.ts Outdated

panteliselef added 2 commits May 10, 2024 13:51

Merge branch 'main' into elef/sdk-1698-node-sdk-failed-to-resolve-jwk…

558dab6

…-during-verification

fix(backend): Drop the InvalidSecretKey error

1dec021

clerk-cookie removed the backend label May 10, 2024

panteliselef added 2 commits May 10, 2024 14:29

fix(clerk-sdk-node): Support verifyToken with optional 2nd param

9d852ad

Merge branch 'main' into elef/sdk-1698-node-sdk-failed-to-resolve-jwk…

cabc23a

…-during-verification

dimkl reviewed May 10, 2024

View reviewed changes

dimkl approved these changes May 10, 2024

View reviewed changes

panteliselef commented May 10, 2024

View reviewed changes

panteliselef added 2 commits May 10, 2024 20:24

test(clerk-sdk-node): Add test to verify the change

b9c92b8

Merge branch 'main' into elef/sdk-1698-node-sdk-failed-to-resolve-jwk…

1afcdcd

…-during-verification

panteliselef force-pushed the elef/sdk-1698-node-sdk-failed-to-resolve-jwk-during-verification branch from fd50eea to 1afcdcd Compare May 10, 2024 17:24

Merge branch 'main' into elef/sdk-1698-node-sdk-failed-to-resolve-jwk…

5782b00

…-during-verification

panteliselef enabled auto-merge (squash) May 10, 2024 18:36

panteliselef merged commit b924022 into main May 10, 2024

panteliselef deleted the elef/sdk-1698-node-sdk-failed-to-resolve-jwk-during-verification branch May 10, 2024 18:44

clerk-cookie mentioned this pull request May 10, 2024

Version Packages #3356

Merged

coderabbitai Bot mentioned this pull request Jul 6, 2026

Refactor server initialization to use HTTP server with socket.io inte… EonEmmanuel/whisper#4

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(clerk-sdk-node): Inherit verifyToken options from clerkClient - #3296

fix(clerk-sdk-node): Inherit verifyToken options from clerkClient#3296
panteliselef merged 8 commits into
mainfrom
elef/sdk-1698-node-sdk-failed-to-resolve-jwk-during-verification

panteliselef commented May 1, 2024 •

edited

Loading

Uh oh!

changeset-bot Bot commented May 1, 2024 •

edited

Loading

Uh oh!

panteliselef commented May 1, 2024

Uh oh!

clerk-cookie commented May 1, 2024

Uh oh!

panteliselef May 1, 2024

Uh oh!

octoper May 1, 2024

Uh oh!

panteliselef May 1, 2024

Uh oh!

dimkl left a comment •

edited

Loading

Uh oh!

panteliselef commented May 9, 2024

Uh oh!

Uh oh!

shadoworion commented May 10, 2024

Uh oh!

dimkl May 10, 2024 •

edited

Loading

Uh oh!

panteliselef May 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

panteliselef commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Type of change

Uh oh!

changeset-bot Bot commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

panteliselef commented May 1, 2024

Uh oh!

clerk-cookie commented May 1, 2024

Uh oh!

panteliselef May 1, 2024

Choose a reason for hiding this comment

Uh oh!

octoper May 1, 2024

Choose a reason for hiding this comment

Uh oh!

panteliselef May 1, 2024

Choose a reason for hiding this comment

Uh oh!

dimkl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

panteliselef commented May 9, 2024

Uh oh!

Uh oh!

shadoworion commented May 10, 2024

Uh oh!

dimkl May 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

panteliselef May 10, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

panteliselef commented May 1, 2024 •

edited

Loading

changeset-bot Bot commented May 1, 2024 •

edited

Loading

dimkl left a comment •

edited

Loading

dimkl May 10, 2024 •

edited

Loading